Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix after_commit on: :destroy not being triggered #404

Closed
wants to merge 1 commit into from

Conversation

npezza93
Copy link

@npezza93 npezza93 commented May 3, 2017

Shoutout to @skorfmann on #228 for for most of this! Fixes #217

Let me know if there are any issues with this approach.

lib/paranoia.rb Outdated
when :destroy
transaction_include_destroy?
when :update
!(transaction_record_state(:new_record) || transaction_include_destroy?)
Copy link
Collaborator

@BenMorganIO BenMorganIO May 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use a x && y instead of a !(x || y)? nvm, I had some of the boolean logic mixed up in my head...

@npezza93
Copy link
Author

npezza93 commented May 5, 2017

Remade this to accommodate rails 5.1 @_trigger_{action}_callback instance methods from this commit: rails/rails@371c083

@rmachielse
Copy link

@npezza93 @BenMorganIO any idea on when this will be merged? We could use the change quite well.

@npezza93
Copy link
Author

npezza93 commented May 25, 2017

@rmachielse ¯\_(ツ)_/¯, I haven't heard anything so I think it's up to @BenMorganIO.

@rmachielse
Copy link

@BenMorganIO what are your thoughts about this?

@rmachielse
Copy link

@BenMorganIO is there any chance this gets merged soon?

@rmachielse
Copy link

@BenMorganIO anything I can do to speed this up?

@rmachielse
Copy link

Is there anyone else who can pick this up? @jhawthorn maybe?

@fenak
Copy link

fenak commented Aug 13, 2017

That'd be very helpful for me as well, would like to see it on upstream. 👍

@koenpunt
Copy link

koenpunt commented Jan 3, 2018

While this probably is the best approach; it differs from how it behaves with Rails 4.2. There a "soft delete" would trigger the update callback, and the destroy callback is called only for really_destroy

@dholdren
Copy link

dholdren commented May 4, 2018

any update on this? I'm running into this now and surprised it hasn't been addressed yet

@JorgeGarciaxyz
Copy link

Any update ?

@ruubyy
Copy link

ruubyy commented Oct 16, 2019

+1

@razum2um
Copy link

@npezza93 thanks for initial PR ❤️

@BenMorganIO will you merge & release it if we'll rebase && fix travis? /cc @jhawthorn

@npezza93
Copy link
Author

npezza93 commented Jan 8, 2021

I believe this is has been fixed with a combo of the latest release and rails >= 5.2

@npezza93 npezza93 closed this Jan 8, 2021
@npezza93 npezza93 deleted the fix-after-commit branch January 8, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

after_commit on: :destroy hooks are broken in 2.1.0
9 participants